Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Delete unused admin, loan, and login templates and endpoints #9722

Merged
merged 6 commits into from
Aug 19, 2024
Merged

Delete unused admin, loan, and login templates and endpoints #9722

merged 6 commits into from
Aug 19, 2024

Conversation

kohantikanath
Copy link
Contributor

Closes #9655

Description:

This PR addresses the removal of unused templates and their associated endpoints, following the investigation of their relevance and usage within the codebase.

Changes:

Deleted the following templates:

  • openlibrary/admin/templates/admin/index.html
  • openlibrary/templates/account/email/forgot.html

Technical

NA

Testing

  • Conducted a code search to confirm that the templates and endpoints slated for deletion are not referenced elsewhere in the codebase.
  • Verified that removing these files does not affect the functionality of related components.

Screenshot

NA

Stakeholders

NA

@scottbarnes
Copy link
Collaborator

Thanks for this, @kohantikanath. Please also remove the loans_admin class found here:

openlibrary/openlibrary/plugins/admin/code.py

Lines 724 to 783 in 46727e2

class loans_admin:
def GET(self):
i = web.input(page=1, pagesize=200)
total_loans = len(web.ctx.site.store.keys(type="/type/loan", limit=100000))
pdf_loans = len(
web.ctx.site.store.keys(
type="/type/loan", name="resource_type", value="pdf", limit=100000
)
)
epub_loans = len(
web.ctx.site.store.keys(
type="/type/loan", name="resource_type", value="epub", limit=100000
)
)
pagesize = h.safeint(i.pagesize, 200)
pagecount = 1 + (total_loans - 1) // pagesize
pageindex = max(h.safeint(i.page, 1), 1)
begin = (pageindex - 1) * pagesize # pagecount starts from 1
end = min(begin + pagesize, total_loans)
loans = web.ctx.site.store.values(
type="/type/loan", offset=begin, limit=pagesize
)
stats = {
"total_loans": total_loans,
"pdf_loans": pdf_loans,
"epub_loans": epub_loans,
"bookreader_loans": total_loans - pdf_loans - epub_loans,
"begin": begin + 1, # We count from 1, not 0.
"end": end,
}
# Preload books
web.ctx.site.get_many([loan['book'] for loan in loans])
return render_template(
"admin/loans",
loans,
None,
pagecount=pagecount,
pageindex=pageindex,
stats=stats,
)
def POST(self):
i = web.input(action=None)
# Sanitize
action = None
actions = ['updateall']
if i.action in actions:
action = i.action
if action == 'updateall':
borrow.update_all_loan_status()
raise web.seeother(web.ctx.path) # Redirect to avoid form re-post on re-load

You'll also want to remove the reference to that class found around line 999.

I'm still not sure about whether /account/email/forgot is in use (perhaps linked to from archive.org?), but for now, as part of this PR, since we're proposing deleting the template, let's delete the endpoint too:

openlibrary/openlibrary/plugins/upstream/account.py

Lines 676 to 699 in 46727e2

class account_ol_email_forgot(delegate.page):
path = "/account/email/forgot"
def GET(self):
return render_template('account/email/forgot')
def POST(self):
i = web.input(username='', password='')
err = ""
act = OpenLibraryAccount.get(username=i.username)
if act:
if OpenLibraryAccount.authenticate(act.email, i.password) == "ok":
return render_template('account/email/forgot', email=act.email)
else:
err = "Incorrect password"
elif valid_email(i.username):
err = "Please enter a username, not an email"
else:
err = "Sorry, this user does not exist"
return render_template('account/email/forgot', err=err)

@scottbarnes scottbarnes added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Aug 10, 2024
@github-actions github-actions bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Aug 10, 2024
@kohantikanath
Copy link
Contributor Author

Hi @scottbarnes,

I have made the changes in the PR, can you review it?

@scottbarnes
Copy link
Collaborator

scottbarnes commented Aug 10, 2024 •
edited
Loading

Thanks, @kohantikanath. It looks as if the /account/email/forgot pathway has been unused since Open Library switched over to requiring an Internet Archive account for authentication, circa #526 and #530.

Though https://openlibrary.org/account/email/forgot still works, I can't see what the utility is, since the information it gives can't be used for login.

Still, perhaps we suggest people use this form to look up their Open Library account email for some reason.

@mekarpeles and @seabelis, to your knowledge, is there any reason we'd want patrons to be able to look up their Open Library email via https://openlibrary.org/account/email/forgot? If not, I'd like to delete the template and endpoint so there are fewer things to maintain.

@scottbarnes
Copy link
Collaborator

@kohantikanath, it looks as if the multiple mentions of openlibrary/templates/account/email/forgot.html in static/css/legacy.less can also be deleted, assuming @mekarpeles and @seabelis agree the endpoint is no longer used. See e.g.

openlibrary/static/css/legacy.less

Line 613 in 431e274

// openlibrary/templates/account/email/forgot.html

@kohantikanath
Copy link
Contributor Author

Hi @scottbarnes,

I have made the changes in the PR, can you review it?

@codecov-commenter
Copy link

codecov-commenter commented Aug 12, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 16.70%. Comparing base (ce16a79) to head (7a7b701).
Report is 223 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #9722      +/-   ##
==========================================
+ Coverage   16.06%   16.70%   +0.64%     
==========================================
  Files          90       90              
  Lines        4769     4824      +55     
  Branches      832      841       +9     
==========================================
+ Hits          766      806      +40     
- Misses       3480     3494      +14     
- Partials      523      524       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@scottbarnes scottbarnes force-pushed the fixes/delete-unused-templates-and-endpoints branch from d2a783f to 7a7b701 Compare August 19, 2024 17:39
@scottbarnes scottbarnes added the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Aug 19, 2024
scottbarnes
scottbarnes approved these changes Aug 19, 2024
View reviewed changes
Copy link
Collaborator

@scottbarnes scottbarnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks for this, @kohantikanath.

@scottbarnes scottbarnes removed the On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing label Aug 19, 2024
@scottbarnes scottbarnes merged commit 8ab2107 into internetarchive:master Aug 19, 2024
5 checks passed
@scottbarnes scottbarnes changed the title Fix(#9655) Deleted unused templates and endpoints Delete unused admin, loan, and login templates and endpoints Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottbarnes scottbarnes scottbarnes approved these changes

Assignees

@scottbarnes scottbarnes

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Delete unused templates and endpoints
3 participants
@kohantikanath @scottbarnes @codecov-commenter